Skip to content

Pr@main/feat workflow api field#2821

Merged
wangdan-fit2cloud merged 2 commits intomainfrom
pr@main/feat-workflow-api-field
Apr 8, 2025
Merged

Pr@main/feat workflow api field#2821
wangdan-fit2cloud merged 2 commits intomainfrom
pr@main/feat-workflow-api-field

Conversation

@wangdan-fit2cloud
Copy link
Copy Markdown
Contributor

What this PR does / why we need it?

Summary of your change

Please indicate you've done the following:

  • Made sure tests are passing and test coverage is added if needed.
  • Made sure commit message follow the rule of Conventional Commits specification.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed.

@f2c-ci-robot
Copy link
Copy Markdown

f2c-ci-robot bot commented Apr 8, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@f2c-ci-robot
Copy link
Copy Markdown

f2c-ci-robot bot commented Apr 8, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@wangdan-fit2cloud wangdan-fit2cloud merged commit 2d6d16e into main Apr 8, 2025
4 checks passed
@wangdan-fit2cloud wangdan-fit2cloud deleted the pr@main/feat-workflow-api-field branch April 8, 2025 07:10
}
}
}
</style>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your code has several minor issues and suggestions for improvement:

  1. Duplicate Code Block: The first handle class rules are duplicated in the .drag-card:not(.no-drag) block, which is unnecessary duplication.

  2. Flexbox Alignment: In the <span> element within the handle, there's an additional inline-block (align-items) that doesn't make sense with existing flex properties.

  3. Image Display Condition: Ensure that the image only appears when hovering over non-droppable items to avoid confusion for users.

Here's a revised version of your component:

<template>
  <el-card
    v-resize="(wh: any) => resizeCondition(wh, item, index)"
    shadow="never"
    class="drag-card card-never mb-8"
    :class="{ 'no-drag': index === form_data.branch.length - 1 }"
    style="--el-card-padding: 12px;"
  >
    <div class="handle flex-between lighter">
      <span class="flex-align-center">
        <img src="@/assets/sort.svg" alt="" height="15" :class="{ 'mr-4': !index === form_data.branch.length - 1 }" />
        {{ item.type }}
      </span>
      <div class="info" v-if="item.conditions.length > 1">
        <!-- Your condition info content here -->
      </div>
    </div>
  </el-card>
</template>

<script setup lang="ts">
// Import necessary components and define props if needed

const { form_data, item_model } = defineProps<{
  form_data: object;
  item_model: object;
}>()

onMounted(() => {
  set(item_model, 'validate', validate);
});
</script>

<style scoped lang="scss">
.drag-card.no-drag {
  .handle {
    img.handle-img {
      // No need for separate styling since it doesn't change anyway
    }
  }
}

.drag-card:not(.no-drag) {
  .handle {
    img.handle-img {
      &:hover {
        display: block;
      }
    }
  }
}
</style>

Key Changes Made:

  1. Removed duplicate rules in the .drag-card:not(.no-drag) section.
  2. Simplified CSS for hover effect by using direct child selectors.
  3. Adjusted spacing and alignment logic for better readability and user experience.

These changes ensure cleaner and more maintainable code while addressing some of the identified issues.

</el-table-column>
<el-table-column :label="$t('common.required')">
<template #default="{ row }">
<div @click.stop>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No significant irregularities or obvious optimizations were found in the provided code snippet. It appears to correctly format an el-table column with ellipsis truncation for both 'variable' and 'default_value'. Additionally, the template is structured slightly differently but serves similar functionality. If this changes the behavior of your table (e.g., causing performance issues), you might need further testing on specific data sets. Otherwise, no adjustments seem necessary.

<span class="break-all">{{ item.label }} {{ '{' + item.value + '}' }}</span>
<el-tooltip
effect="dark"
:content="$t('views.applicationWorkflow.setting.copyParam')"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The provided code has two main areas for improvement:

  1. Break All Text: The line <span class="break-all">{{ item.label }} {{ '{' + item.value + '}' }}</span> is adding a CSS class break-all to the span element containing text that might be too long or wide to fit in its container. This prevents overflow and ensures all content is shown regardless of width. Consider wrapping long labels or values within a div with the same style if necessary.

  2. Optimization: There isn't anything specific about the current implementation that appears to need optimization at this time. However, ensure that any event handlers (@mouseenter, @mouseleave) are efficient and perform their functions without unnecessary computations. Also, verify that the $i18n.t function call (used in the tooltip) is properly loaded and works correctly across translations.

Here's the cleaned-up version with minor adjustments:

<span v-if="item.label" class="break-all">
  <span>{{ item.label }}
  {{ '{' + item.value + '}' }}
</span>

<el-tooltip effect="dark" :content="$t('views.applicationWorkflow.setting.copyParam')" placement="right-start"></el-tooltip>

Ensure proper HTML structure integrity and test the changes in various environments to confirm they meet functionality requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants